-
Notifications
You must be signed in to change notification settings - Fork 1
Yash/admin profile view #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…odo is to make the edit/delete/deactivate buttons actually do something, add back protectedpage and endpoint auth requirements so only admins can view this, testing for more cases (eg. volunteers without cancer or without loved ones)
…, wire up delete and deactivate buttons, edit functionality for the profile summary only (edits work for the availability and cancer experience parts of the screen.
…ities with days of week, added unit tests, updated other parts to use new table
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| user_dict = { | ||
| **{c.name: getattr(updated_user, c.name) for c in updated_user.__table__.columns}, | ||
| 'availability': availability_templates, | ||
| 'role': updated_user.role, | ||
| 'user_data': updated_user.user_data, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep User names in sync when editing profile
The new update_user_data_by_id endpoint only writes name fields to UserData, then builds the response (and subsequent GET /users/{id} values) from updated_user.__table__.columns, so first_name/last_name returned to the frontend never change. Admin profile edits that call this endpoint will update the profile summary (which reads user_data) but the header/directory still show the old names from users. Either persist the name changes on the User model or source the response fields from user_data so edits are reflected consistently.
Useful? React with 👍 / 👎.
| POSTGRES_DATABASE_URL = os.getenv("POSTGRES_TEST_DATABASE_URL") | ||
| if not POSTGRES_DATABASE_URL: | ||
| raise RuntimeError( | ||
| "POSTGRES_TEST_DATABASE_URL is not set. Please export a Postgres URL, e.g. " | ||
| "postgresql+psycopg2://postgres:postgres@db:5432/llsc_test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip Postgres-only availability tests when DB env absent
This test module raises a RuntimeError at import if POSTGRES_TEST_DATABASE_URL is not set, so running pytest without that environment variable aborts the entire suite instead of skipping as other modules (e.g., test_match_service.py) do. In environments without a Postgres test DB configured, every test run will now fail before executing. Add the same pytest.skip(...) guard or mark to allow the suite to run when the DB isn’t available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one doesn't need to be fixed
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not volunteer_tz: | ||
| self.logger.warning( | ||
| f"Volunteer {volunteer.id} has no timezone set. Interpreting availability templates as UTC." | ||
| ) | ||
| volunteer_tz = timezone.utc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unknown timezones treated as UTC in availability projection
Volunteer availability is projected using get_timezone_from_abbreviation, but when that lookup fails the service silently falls back to UTC (backend/app/services/implementations/match_service.py lines 803-807). The admin profile UI collects timezone as a free-text input (ProfileSummary) rather than one of the mapped abbreviations, so entries like “Eastern” or “America/Toronto” will not resolve and the volunteer’s templates will be interpreted in UTC, shifting all suggested time blocks by their timezone offset. Any volunteer whose timezone string isn’t one of the hardcoded Canadian abbreviations will therefore surface incorrect suggested call times to participants.
Useful? React with 👍 / 👎.
…h confirmation and success modals
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const updatedUser = await updateUserData(userId as string, { | ||
| firstName: profileEditData.firstName, | ||
| lastName: profileEditData.lastName, | ||
| dateOfBirth: profileEditData.dateOfBirth, | ||
| phone: profileEditData.phone, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank birthday sent as invalid date
When saving the profile summary, dateOfBirth is forwarded directly from the form to updateUserData even when the field is empty. The backend UserDataUpdateRequest.date_of_birth accepts a date or null, so an empty string fails validation with a 422 response and blocks saving any changes for users whose birthday is unset or after clearing it. Normalize empty dates to null/omit the field before calling the API so edits can succeed.
Useful? React with 👍 / 👎.
## Notion ticket link <!-- Please replace with your ticket's URL --> [Ticket Name](https://www.notion.so/uwblueprintexecs/Task-Board-db95cd7b93f245f78ee85e3a8a6a316d) ## Implementation description * Replaced ad-hoc available_time records with recurring `AvailabilityTemplate`s: new model + migrations, schema updates, timezone utilities, and service logic that creates/deletes template slots, validates input, and projects active templates when Hydrating users/admin lists. * Added an admin `PATCH /users/{id}/user-data` flow backed by `UserDataUpdateRequest`, expands `UserService` to materialize treatments/ experiences/loved-one info, and ensures responses always include active availability templates; extended seeds and helper scripts to create/check/ undo match data plus availability seeding docs. * Hardened `MatchService` to only attach suggestions for volunteers with templates and to compare template weekdays in the volunteer’s timezone before converting to UTC; added comprehensive unit coverage (`test_availability_service`, `test_match_service`, `test_match_service_timezone`, `test_user_data_update`). * Built the admin user profile experience (`/admin/users/[id]`): navigation sidebar, profile summary card, editable cancer experience/loved-one sections, availability grid editor with drag-select, success messaging, and supporting hooks (`useUserProfile`, `useProfileEditing`, `useAvailabilityEditing`, `useIntakeOptions`) plus Chakra-based UI components. * Updated the admin directory/header to route into the new profile view, expanded API clients/types/utilities to handle user-data patching + availability templates, and added UI polish (single-select dropdown upgrades, user profile formatting helpers, date utilities, success banner, react- datepicker). ## Steps to test 1. `docker-compose up -d` then run `cd backend && pdm run alembic upgrade head` to apply the availability template migrations. 2. `cd backend && pdm run dev` and `cd frontend && npm run dev`; sign in to the admin portal. 3. Visit `/admin/directory`, open any volunteer, and verify the profile page renders summary, profile, cancer, loved-one, and availability sections with existing data. 4. Click “Edit” in Profile Summary to update personal/demographic fields, save, and confirm the success toast plus persisted values via a hard refresh; repeat for cancer and loved-one sections (including clearing dates). 5. Use “Edit Availability” to drag-select new slots, save, and confirm the grid + subsequent reload reflect the template changes (verify via network tab hitting `/availability`). 6. Optional back-end verification: run `cd backend && pdm run tests backend/tests/unit/test_availability_service.py backend/tests/unit/ test_match_service_timezone.py backend/tests/unit/test_user_data_update.py` to exercise the new logic, and use the helper scripts (`create_test_matches.py`, `check_matches.py`, `undo_test_changes.py`) to inspect match suggestion behavior. ## What should reviewers focus on? * **Migrations & data model:** ensure the `availability_templates` migration, seeds, and new helper scripts won’t clobber production data when rolled out. * **User data patching:** confirm `UserService.update_user_data_by_id` correctly hydrates treatments/experiences and returns availability templates so the admin UI state stays consistent. * **Matching/timezone logic:** double-check `_attach_initial_suggested_times` now uses volunteer-local weekdays and that suggested blocks remain correct across timezones. * **Admin UI flows:** exercise the new hooks/components for editing profile sections and availability (loading states, drag interactions, and API payloads) for both happy paths and cancellations. * **API client/type updates:** watch for any regressions in other consumers now that `UserResponse` carries availability templates and the auth API client maps the new schemas. ## Checklist - [ ] My PR name is descriptive and in imperative tense - [ ] My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits - [ ] I have run the appropriate linter(s) - [ ] I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR
Notion ticket link
Ticket Name
Implementation description
AvailabilityTemplates: new model + migrations, schema updates, timezoneutilities, and service logic that creates/deletes template slots, validates
input, and projects active templates when Hydrating users/admin lists.
PATCH /users/{id}/user-dataflow backed byUserDataUpdateRequest, expandsUserServiceto materialize treatments/experiences/loved-one info, and ensures responses always include active
availability templates; extended seeds and helper scripts to create/check/
undo match data plus availability seeding docs.
MatchServiceto only attach suggestions for volunteerswith templates and to compare template weekdays in the volunteer’s
timezone before converting to UTC; added comprehensive unit
coverage (
test_availability_service,test_match_service,test_match_service_timezone,test_user_data_update)./admin/users/[id]): navigationsidebar, profile summary card, editable cancer experience/loved-one sections,
availability grid editor with drag-select, success messaging, and supporting
hooks (
useUserProfile,useProfileEditing,useAvailabilityEditing,useIntakeOptions) plus Chakra-based UI components.expanded API clients/types/utilities to handle user-data patching +
availability templates, and added UI polish (single-select dropdown upgrades,
user profile formatting helpers, date utilities, success banner, react-
datepicker).
Steps to test
docker-compose up -dthen runcd backend && pdm run alembic upgrade headto apply the availability template migrations.cd backend && pdm run devandcd frontend && npm run dev; sign in tothe admin portal.
/admin/directory, open any volunteer, and verify the profile pagerenders summary, profile, cancer, loved-one, and availability sections with
existing data.
save, and confirm the success toast plus persisted values via a hard refresh;
repeat for cancer and loved-one sections (including clearing dates).
grid + subsequent reload reflect the template changes (verify via network tab
hitting
/availability).cd backend && pdm run tests backend/tests/unit/test_availability_service.py backend/tests/unit/ test_match_service_timezone.py backend/tests/unit/test_user_data_update.pyto exercise the new logic, and use the helper scripts
(
create_test_matches.py,check_matches.py,undo_test_changes.py) toinspect match suggestion behavior.
What should reviewers focus on?
availability_templatesmigration,seeds, and new helper scripts won’t clobber production data when rolled out.
UserService.update_user_data_by_idcorrectly hydrates treatments/experiences and returns availability templates
so the admin UI state stays consistent.
_attach_initial_suggested_timesnow uses volunteer-local weekdays and that suggested blocks remain correct
across timezones.
sections and availability (loading states, drag interactions, and API
payloads) for both happy paths and cancellations.
now that
UserResponsecarries availability templates and the auth APIclient maps the new schemas.
Checklist
are atomic and trivial commits are squashed or fixup'd into non-trivial
commits
background knowledge on this PR or who will be building on top of this PR